rpc: resolve eth_getLogs and trace_filter tags on the committed view - #23279
Merged
Conversation
Sonar go:S3776 flagged getLogsV3 at 64, above the 60 limit. Extract the maxResults-capped log append, which had three near-identical copies in the loop, into appendErigonLogs, reusing the existing types.Logs.ToErigonLogs conversion. Extract the bor state sync event lookup into borStateSyncLogs, mirroring the sibling borReceiptForBlock. Flatten the bor branch with an early continue and drop the unreachable header re-fetch: the first Next() of MapTxNum2BlockNumIter always reports blockNumChanged, so header is non-nil past that block. In the test package, share the overlay-race chain setup between the two testers and replace the unused mockBridgeReader with a configurable one. Cognitive complexity of getLogsV3 goes from 78 to 44 by gocognit.
lupin012
marked this pull request as ready for review
August 15, 2026 16:57
AskAlexSharov
approved these changes
Aug 16, 2026
AskAlexSharov
left a comment
AskAlexSharov
left a comment
Collaborator
There was a problem hiding this comment.
StateCache doesn't store inverted_index. SharedDomains writing II to ETL. Means RPCD can't read kv.LogAddrIdx - for data which are not in db.
eth_getLogs -> applyFiltersV3 func will not see data which is not in db. I don't know - is it blocker for this pr or not.
Also: solve data integrity bug by adding nil parameter to some func - looks weird. We have tx object which responsibility - to guarantee consistent data view. Other objects around - must not break it. (but it's another story)
lupin012
added a commit
that referenced
this pull request
Aug 17, 2026
overlay_race_test.go: keep the branch's overlayAheadHarness and adopt main's insertOverlayRaceChain helper; the three tests from #23279 now build their API through the harness.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #23194.
Same class of bug as #23193: block tags resolved on the overlay view while the data scan reads the committed view. During an FCU background-commit window,
eth_getLogsonlatestfailed transiently andtrace_filtersilently omitted the head block.Changes
eth_getLogs: resolve user tags withnilfilters, on the same committed view as thelatestbaseline and the log scantrace_filter: same, plusCheckBlockExecutedon an explicittoBlockso a not-yet-executed block errors instead of being silently clamped awaydebug_getModifiedAccountsByHash: add thestartNum > latestBlockguardits ByNumber twin already has
Trade-off (as accepted in #23193):
pendingresolves to the latest executed block.Second commit: getLogsV3 complexity
SonarCloud flagged
getLogsV3on this PR (go:S3776, 64 against the 60 allowed). Purerefactor, no behaviour change: the three duplicated maxResults-capped append loops become
appendErigonLogs, the state-sync lookup becomesborStateSyncLogs. 78 → 44 by gocognit,Sonar issue now closed as fixed.
Notes
nil-filters hunks as part of a broader view-consistency pass; whichever merges second rebases trivially.BeginTemporalRoWithOverlay); migrating these call sites to it is the agreed follow-up — this PR keeps the endpoints correct in the meantime.Testing
New tests in
overlay_race_test.go, reusing the overlay helper introduced by #23193 plus a newnewHeaderAheadTesterhelper (canonical header committed one past execution progress). All verified red before the fix and green after:TestGetLogs_UsesCommittedFromTag/TestGetLogs_UsesCommittedToTagTestTraceFilter_UsesCommittedFromTagTestTraceFilter_FutureToBlockErrorsTestGetModifiedAccountsByHash_FutureStartBlockErrorsThe refactor commit is behaviour-preserving, so the existing
TestGetLogs_*tests are itssafety net;
TestAppendErigonLogsandTestBorStateSyncLogs_NoEvents/_EventsErrorpinthe extracted helpers